Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: resolve macros/derived-traits from crates w/ scopes rather than globally #2198

Merged
merged 8 commits into from
May 27, 2024

Conversation

donaldguy
Copy link
Contributor

@donaldguy donaldguy commented May 27, 2024

i.e. remove depreciated use of #[macro_import] extern crate

to make this more sane for reviewing, I did commits by crate. (and can confirm that each one builds cleanly)

I'm pretty sure the contracts, eyre, indoc, and strum ones are worth taking for overall clarity-of-provenance.


I have more mixed feelings on the utility of scoping pretty_assertions and insta assertions.

I, in fact, tried (at least) doing:

pub use pretty_assertions::{assert_eq, assert_ne, assert_str_eq};

in src/test.rs and then it was gonna e.g. use crate::test::* or similar (crate::test::assertions::*?), but that made stuff angry about possible ambiguity (based on implications/expansion of #[test]?)

The "correct" way to do this, I think, then is to rather overrider/replace the #[test] attribute itself with e.g. a #[mise_test] (for the insta ones maybe a #[snapshot_test]) or similar


I was working on a larger refactor

(trying to disentangle the cli— in particular ForgeArg, ToolArg, ToolVersionType (to limit down cli-layer usage to the FromStr-ing?); ui (SingleReport and TreeItem traits) and most of the env crates from the rest, with a view towards maybe doing a separate lib.rs and minimal main.rs)

but I wore myself out and stepped back to start with offering this [and now probably lose interest for at least several days]

¯\(ツ)

@jdx jdx merged commit 9a9924e into jdx:main May 27, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants